fix(fs): expandGlob/expandGlobSync - match non-glob path segments containing escaped glob chars
#6788
Conversation
dsherret
changed the title
expandGlob/expandGlobSync - match non-glob path segments containing escaped glob chars
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6788 +/- ##
=======================================
Coverage 94.09% 94.09%
=======================================
Files 578 578
Lines 42633 42655 +22
Branches 6779 6782 +3
=======================================
+ Hits 40114 40135 +21
- Misses 2468 2469 +1
Partials 51 51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where expandGlob and expandGlobSync functions were not properly handling escaped glob characters in non-glob path segments. The fix ensures that when path segments contain escaped glob characters (like \[ or \]), they are correctly unescaped when building the fixed root path.
- Added
unescapeGlobSegmenthelper function to remove escape characters from path segments - Updated both
expandGlobandexpandGlobSyncto unescape non-glob segments before joining them to the fixed root - Added comprehensive tests for both async and sync versions with escaped bracket scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| fs/expand_glob.ts | Implements the core fix by adding unescaping logic for non-glob path segments |
| fs/expand_glob_test.ts | Adds test cases to verify escaped bracket handling in directory names |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@kt3k I updated this PR based on the AI feedback. It was too simplistic before I think |
fs/expand_glob.ts
Outdated
| } | ||
|
|
||
| const globEscapeChar = Deno.build.os === "windows" ? "`" : `\\`; | ||
| const globMetachars = "*?{}[]"; |
There was a problem hiding this comment.
shouldn't we also include ()|+@!? (Our glob matcher supports extended glob like @(foo|bar).txt
There was a problem hiding this comment.
Good catch. I updated it. I wonder if we should specifically test this unescapeGlobSegment function.
2 participants
Closes #6787